Skip to content

perf: reduce the amount of code converted to AST #354

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Feb 7, 2025
Merged

perf: reduce the amount of code converted to AST #354

merged 1 commit into from
Feb 7, 2025

Conversation

vicb
Copy link
Contributor

@vicb vicb commented Feb 7, 2025 •
edited
Loading

We only patch files (vs the whole bundle) to patch instrumentation

Changes in this PR:

  • move the instrumentation patch to the build and on files vs the whole bundle
  • create an ESBuild plugin to apply multiple content modifications
  • move one plugin to the plugins folder
  • drop ts-morph

I did that because users are reporting OOM on DIscord while patching - the PR should reduces CPU and memory usage by minimizing parsing

@wesbos said on Discord:

  • "locally the memory usage went from 3.5gb to 700mb. Trying a deploy now"
  • "That worked! And it seems shaved off ~60s from my build too. Thank you!" (fixed a broken build on Workers build)

@vicb
perf: reduce the amount of code converted to AST
e895222 
We only patch files (vs the whole bundle) to patch instrumentation
@changeset-bot changeset-bot
Copy link

changeset-bot bot commented Feb 7, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: e895222

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@opennextjs/cloudflare Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new pkg.pr.new
Copy link

pkg-pr-new bot commented Feb 7, 2025 •
edited
Loading

Open in Stackblitz

pnpm add https://pkg.pr.new/@opennextjs/cloudflare@354

commit: e895222

@wesbos
Copy link
Contributor

wesbos commented Feb 7, 2025

This PR fixed my memory issues - thank you!

@vicb
Copy link
Contributor Author

vicb commented Feb 7, 2025

This PR fixed my memory issues - thank you!

I have a lot of memory issues, I should try :)

james-elicx
james-elicx approved these changes Feb 7, 2025
View reviewed changes
Copy link
Collaborator

@james-elicx james-elicx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The content updater for multiple plugins is a pretty neat idea. Nice, looks good to me.

@vicb vicb merged commit a19b34d into main Feb 7, 2025
7 checks passed
@vicb vicb deleted the load-inst branch February 7, 2025 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@james-elicx james-elicx james-elicx approved these changes

@dario-piotrowicz dario-piotrowicz Awaiting requested review from dario-piotrowicz

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vicb @wesbos @james-elicx